Skip to content

[contoso] Move/copy readme.md for folder structure v2 #34677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

mikeharder
Copy link
Member

@mikeharder mikeharder commented May 14, 2025

Changelog

  1. data-plane/readme.md moved one level down to data-plane/Azure.Contoso.WidgetManager/readme.md
  2. resource-manager/readme.md copied to resource-manager/Microsoft.Contoso/readme.md
    • resource-manager/readme.md: Represents "ARM Schema Manifest" file (under RP namespace in final state)
    • resource-manager/Microsoft.Contoso/readme.md: Represents SDK/service level file (under RP namespace and service in final state)

Related

Copy link

openapi-pipeline-app bot commented May 14, 2025

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This PR targets either the main branch of the public specs repo or the RPSaaSMaster branch of the private specs repo. These branches are not intended for iterative development. Therefore, you must acknowledge you understand that after this PR is merged, the APIs are considered shipped to Azure customers. Any further attempts at in-place modifications to the APIs will be subject to Azure's versioning and breaking change policies. Additionally, for control plane APIs, you must acknowledge that you are following all the best practices documented by ARM at aka.ms/armapibestpractices. If you do intend to release the APIs to your customers by merging this PR, add the PublishToCustomers label to your PR in acknowledgement of the above. Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
  • ❌ The required check named Swagger Avocado has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

Copy link

openapi-pipeline-app bot commented May 14, 2025

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

Copy link

github-actions bot commented May 14, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Language API Review for Package
Swagger Contoso.Widget-Microsoft.Contoso

@mikeharder mikeharder requested a review from weshaggard May 14, 2025 06:32
@@ -0,0 +1,48 @@
# containerstorage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this readme related to the one in the WidgetManagment folder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readme.md file in the Contoso.Widget folder is the union of the content in each service name folder's readme.md file. This union file is required by RPSaaS. I just learned yesterday that they will get rid of the need for this file "someday".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be an import of the other files instead of a copy of the content?

Also, I suspect there may be other tools that require the readme at this level as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the RPaaS schema validator will work if the top-level readme file imports (references) the lower-level readme files. I don't think other tools need this top-level readme because I had one team remove it entirely (because I didn't know about the RPaaS schema validation checker) and the schema validation checker was the only thing that broke.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also know that docs & SDKs will refer to the lower-level readmes and not the top-level readme.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is splitting (and re-merging) the readmes the entire point of folder structure v2? For example, this spec already has two levels of folders under resource-manager, but only a single readme.md directly under resource-manager.

https://github.com/Azure/azure-rest-api-specs/tree/main/specification/compute/resource-manager

So, while this spec already has their swaggers using the v2 folder structure, their readme.md is still using the v1 structure? Is this why this spec works with "schema validation" today, but requires an extra (merged) readme after the readmes are split and moved down into sub-folders?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The high priority goal is to make each service self-contained.
So, each service has its own folder and ALL things related to this service are in that folder: tsp files, preview/stable swaggers, readme.md files, etc. And from these files, docs & SDKs are produced. If a service is later retired, then we can just delete the service folder and everything related to that service is deleted in one fell swoop.

For control-plane services, they are under an extra folder: the RP namespace folder (eg: Microsoft.Compute). data-plane doesn't have this extra folder. And, control-plane services all need this extra readme.md file because of RPaaS schema validation which validates all operations within the RP namespace regardless of how the operations are split across multiple services. The RPaaS team told me that they hope to get rid of the requirement for this extra readme.md file in the future so this is required as a stop gap until then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, so it's what I said in my last paragraph. Some specs already in main split their swaggers into self-contained folders, but they still use a single readme.md at the top-level of resource-manager.

For these specs (like compute), the only change in v2 will be splitting the existing readme.md into multiple files, one per "service", and one extra readme per "RP namespace" (hopefully temporary).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will all the content need to be duplicated in those split readmes? Or only part of the info? We need to figure out what is needed in each type of readme to ensure the duplication and confusion is minimal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each service readme.md has only what's needed for that specific service. The readme inside the RP namespace folder is the union of the other service readme files. The service readme should only have paths in it to swagger files in the same service and to nothing outside of that service.

@mikeharder mikeharder changed the title [contoso] Move swagger and readme for folder structure v2 [contoso] Update readme.md for folder structure v2 May 21, 2025
@mikeharder mikeharder changed the title [contoso] Update readme.md for folder structure v2 [contoso] Move/duplicate readme.md for folder structure v2 May 21, 2025
@mikeharder mikeharder changed the title [contoso] Move/duplicate readme.md for folder structure v2 [contoso] Move/copy readme.md for folder structure v2 May 21, 2025
@AzureRestAPISpecReview AzureRestAPISpecReview removed TypeSpec Authored with TypeSpec BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required new-rp-namespace labels May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants